Skip to content

🎨 web-server api: ordering parameters and simplified openapi specs for complex query parameters #6737

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 42 commits into from
Nov 18, 2024
Merged

🎨 web-server api: ordering parameters and simplified openapi specs for complex query parameters #6737

merged 42 commits into from
Nov 18, 2024

Conversation

@pcrespov
Copy link
Member

@pcrespov pcrespov commented Nov 15, 2024 •
edited
Loading

What do these changes do?

ReDoc Swagger UI
Unifies and simplifies ordering queries. For instance, list folders is displayed as
image
from this coded openapi specification

@router.get(
    "/folders",
    response_model=Envelope[list[FolderGet]],
)
async def list_folders(
    _q: Annotated[as_query(FolderListQueryParams), Depends()],
):
    ...

♻️ Unified ordering query parameters

  • SEE packages/models-library/src/models_library/rest_ordering.py
  • Introduced a factory function to streamline the handling of ordering parameters, ensuring flexibility for customization per case.
  • Refactored existing resources (folders, projects, ...) to adopt the unified approach for defining ordering parameters.
  • Established a naming convention for models: $(ResourceName)$(OperationName)OrderQueryParams (e.g., FoldersListOrderQueryParams).

♻️ Simplified handling of complex query parameters (i.e. JSON-encoded)

  • Unified the logic with a single class for both OpenAPI specification generation and request validation.
    • Includes the helper function as_query for ease of use
  • Improved implementation in api/specs/web-server to reduce complexity and potential errors.. E.g. openapi specs are simply defined using one model per "group" of parameters
  • Note that it uses dependency injection shortcuts to expand all fields in the model as (path _p/ query _q) parameters.

Related issue/s

  • Preparations to include ordering in ITISFoundation/private-issues#6

How to test

Driving test

cd pakcages/models-library
make install-dev
pytest -vv tests/test_rest_ordering

Dev-ops

None

@pcrespov pcrespov self-assigned this Nov 15, 2024
@codecov
Copy link

codecov bot commented Nov 15, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 94.68085% with 10 lines in your changes missing coverage. Please review.

Project coverage is 85.70%. Comparing base (5f70a0d) to head (d85312a).
Report is 1 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #6737      +/-   ##
==========================================
- Coverage   88.33%   85.70%   -2.63%     
==========================================
  Files        1440     1573     +133     
  Lines       58398    63309    +4911     
  Branches     1405     2125     +720     
==========================================
+ Hits        51583    54257    +2674     
- Misses       6615     8739    +2124     
- Partials      200      313     +113
Flag Coverage Δ
integrationtests ?
unittests 85.70% <94.68%> (-0.80%) ⬇️
Components Coverage Δ
api 79.60% <ø> (ø)
pkg_aws_library 93.38% <ø> (ø)
pkg_dask_task_models_library 96.87% <ø> (ø)
pkg_models_library 91.93% <85.71%> (-0.09%) ⬇️
pkg_notifications_library 83.79% <ø> (ø)
pkg_postgres_database 87.33% <ø> (ø)
pkg_service_integration 71.44% <ø> (ø)
pkg_service_library 76.54% <100.00%> (∅)
pkg_settings_library 91.42% <ø> (ø)
pkg_simcore_sdk 65.69% <ø> (-19.58%) ⬇️
agent 96.98% <ø> (ø)
api_server 89.88% <ø> (ø)
autoscaling 95.22% <ø> (ø)
catalog 89.42% <ø> (ø)
clusters_keeper 98.72% <ø> (ø)
dask_sidecar 91.32% <ø> (ø)
datcore_adapter 94.05% <ø> (ø)
director 58.38% <ø> (-0.05%) ⬇️
director_v2 84.32% <ø> (-6.51%) ⬇️
dynamic_scheduler 96.59% <ø> (ø)
dynamic_sidecar 88.83% <ø> (-0.95%) ⬇️
efs_guardian 90.01% <ø> (ø)
invitations 93.51% <ø> (ø)
osparc_gateway_server 39.84% <ø> (-45.58%) ⬇️
payments 92.87% <ø> (ø)
resource_usage_tracker 90.65% <ø> (-0.15%) ⬇️
storage 89.76% <ø> (ø)
webclient ∅ <ø> (∅)
webserver 87.23% <98.46%> (-0.36%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5f70a0d...d85312a. Read the comment docs.

---- 🚨 Try these New Features:

@pcrespov pcrespov added the a:webserver webserver's codebase. Assigning the area is particularly useful for bugs label Nov 15, 2024
@pcrespov pcrespov added this to the Event Horizon milestone Nov 15, 2024
@pcrespov pcrespov changed the title WIP: Is468/rest ordering 🎨 web-server api: ordering parameters and simplified generation of openapi specs for complex query parameters Nov 15, 2024
@pcrespov pcrespov changed the title 🎨 web-server api: ordering parameters and simplified generation of openapi specs for complex query parameters 🎨 web-server api: ordering parameters and simplified openapi specs for complex query parameters Nov 15, 2024
@pcrespov pcrespov force-pushed the is468/rest-ordering branch from 82d2ea8 to 7827e0d Compare November 15, 2024 20:39
@pcrespov pcrespov marked this pull request as ready for review November 15, 2024 20:43
sanderegg
sanderegg approved these changes Nov 18, 2024
View reviewed changes
Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice refactoring.
This is the inverse of parse_as_body/header/param/query methods. nice!
Just a few comments and questions.

matusdrobuliak66
matusdrobuliak66 approved these changes Nov 18, 2024
View reviewed changes
Copy link
Collaborator

@matusdrobuliak66 matusdrobuliak66 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much! It’s really nice. I’ve left a few comments.

@pcrespov pcrespov force-pushed the is468/rest-ordering branch from 2b9b63b to 911f54e Compare November 18, 2024 12:02
@pcrespov pcrespov enabled auto-merge (squash) November 18, 2024 12:04
@pcrespov pcrespov force-pushed the is468/rest-ordering branch from 911f54e to 357ba30 Compare November 18, 2024 15:26
@pcrespov pcrespov force-pushed the is468/rest-ordering branch from 357ba30 to d85312a Compare November 18, 2024 16:16
@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 18, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@pcrespov pcrespov merged commit 625d2cb into ITISFoundation:master Nov 18, 2024
82 of 89 checks passed
@pcrespov pcrespov deleted the is468/rest-ordering branch November 19, 2024 18:31
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Nov 26, 2024
Closed
18 tasks
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Jan 15, 2025
Closed
58 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@giancarloromeo giancarloromeo giancarloromeo approved these changes

@GitHK GitHK GitHK approved these changes

@sanderegg sanderegg sanderegg approved these changes

@matusdrobuliak66 matusdrobuliak66 matusdrobuliak66 approved these changes

@bisgaard-itis bisgaard-itis Awaiting requested review from bisgaard-itis

@odeimaiz odeimaiz Awaiting requested review from odeimaiz

Assignees

@pcrespov pcrespov

Labels

a:webserver webserver's codebase. Assigning the area is particularly useful for bugs

Projects

None yet

Milestone

Event Horizon

Development

Successfully merging this pull request may close these issues.

5 participants

@pcrespov @giancarloromeo @GitHK @sanderegg @matusdrobuliak66